Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure firstOrNew/Create can be called without parameters in Laravel 8+ #157

Merged
merged 4 commits into from
Jul 13, 2021
Merged

Ensure firstOrNew/Create can be called without parameters in Laravel 8+ #157

merged 4 commits into from
Jul 13, 2021

Conversation

caugner
Copy link
Contributor

@caugner caugner commented Jun 23, 2021

Fixes #156.

🟢 Includes 🍒 from #181. Blocked by #182.

@mr-feek
Copy link
Collaborator

mr-feek commented Jun 24, 2021

I think we need to create separate stubs for separate laravel versions? It seems like laravel 7 introduced the ability for attributes to be nullable if I'm reading the docs right. Laravel 6 does not allow that.

We should probably take a similar approach as the symfony plugin: https://github.com/psalm/psalm-plugin-symfony/tree/master/src/Stubs

@mr-feek
Copy link
Collaborator

mr-feek commented Jun 24, 2021

A test would be nice as that would verify the issue I'm bringing up, I believe

@caugner
Copy link
Contributor Author

caugner commented Jun 24, 2021

It seems like laravel 7 introduced the ability for attributes to be nullable if I'm reading the docs right. Laravel 6 does not allow that.

It's a bit inconsistent:

  • Laravel 6 did always require $attributes to be passed.
  • Laravel 7 allowed Builder::firstOrNew() to be called without attributes (but not Builder::firstOrCreate()).
  • Laravel 8 allowed Builder::firstOrCreate(), HasOneOrMany::firstOrNew() and HasOneOrMany::firstOrCreate() to be called without attributes.
  • Laravel 9 will allow BelongsToMany::firstOrNew() and BelongsToMany::firstOrCreate() to be called without attributes (if [9.x] Support calling BelongsToMany::firstOrNew/Create without parameters laravel/framework#37791 gets merged).

@caugner
Copy link
Contributor Author

caugner commented Jun 24, 2021

A test would be nice as that would verify the issue I'm bringing up, I believe

I guess the test would need to have different expectations depending on the Laravel version?

edit: I'm not familiar with behat, but we probably need a condition that restricts a test to a Laravel version, like Given I have Laravel 8 installed.

@caugner
Copy link
Contributor Author

caugner commented Jun 24, 2021

We should probably take a similar approach as the symfony plugin: https://github.com/psalm/psalm-plugin-symfony/tree/master/src/Stubs

Implemented in 7f9994d.

@caugner
Copy link
Contributor Author

caugner commented Jul 6, 2021

@mr-feek Any tips how to add test scenarios that only run with a specific Laravel major version, and that are skipped otherwise? I only found some unconditional @skip annotations.

@mr-feek
Copy link
Collaborator

mr-feek commented Jul 9, 2021

I think it might need a custom codeception module helper...

@weirdan implemented this for determining the version of psalm at runtime during tests.. could do something similar https://github.com/psalm/codeception-psalm-module/blob/master/src/Module.php#L201-L235

@weirdan
Copy link
Member

weirdan commented Jul 9, 2021

Custom step is not required. You can check the version of any package, see release notes for v0.9.0

@mr-feek
Copy link
Collaborator

mr-feek commented Jul 9, 2021

One step ahead of us :) thanks @weirdan !

@caugner
Copy link
Contributor Author

caugner commented Jul 9, 2021

Cherry-picked 1191d0f from #181 to see if the tests pass.

@caugner caugner marked this pull request as draft July 9, 2021 20:47
@caugner caugner changed the title fix: allow firstOrNew/Create to be called without parameters Ensures firstOrNew/Create can be called without parameters in Laravel 8+ Jul 9, 2021
@caugner caugner changed the title Ensures firstOrNew/Create can be called without parameters in Laravel 8+ Ensure that firstOrNew/Create can be called without parameters in Laravel 8+ Jul 9, 2021
@caugner caugner changed the title Ensure that firstOrNew/Create can be called without parameters in Laravel 8+ Ensure firstOrNew/Create can be called without parameters in Laravel 8+ Jul 9, 2021
@caugner caugner marked this pull request as ready for review July 13, 2021 16:33
@caugner caugner marked this pull request as draft July 13, 2021 16:33
@caugner caugner marked this pull request as ready for review July 13, 2021 17:25
@caugner caugner requested a review from mr-feek July 13, 2021 17:29
@mr-feek mr-feek added the fix label Jul 13, 2021
@mr-feek
Copy link
Collaborator

mr-feek commented Jul 13, 2021

thanks!

@mr-feek mr-feek merged commit 5196f8f into psalm:master Jul 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[8.x] allow firstOrCreate() and firstOrNew() without parameters
3 participants